You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It seems that sometimes the 500 error page gets cached for some reason.
We don't want that to happen.
Update:
This can happen for page router if your 500 (or 404) page use getStaticProps.
For every route in page router that throws an exception, the cache-control gets overwritten and set as if it was an SSG route. The problem is that it doesn't remove the previous header that was set (including set-cookie). This could lead to session leakage.
Haven't tested in Next 15, this is probably fixed there since they don't override your cache-control anymore ( It might still happen if you don't set any cache-control for the route )
I added an env variable OPEN_NEXT_DANGEROUSLY_SET_ERROR_HEADERS that if set to true will disable the cache-control override that we do
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Would this be related at all w/ the one user on discord having a random session leak?
It's probably it's error yeah. I need to investigate this more closely but based on what he describes it is likely the issue.
If the set-cookie headers has been set and an error occurs after that then the 500 could be cached.
Still not sure why next would set cache-control on a 500 though, seems very weird
Yea I'm not sure what the use case would be for errors to be cached. The HTTP spec says not to cache 500's.
There is a use case that i can think of. Just to reduce the load on the server when you know that something is broken, you could cache the error for a small amount of time.
Still i don't think it's a good idea, and it might just be that since they load the 500 page from the incremental cache, they assume it is ISR/SSG and apply the cache-control headers.
this is interesting, probably what that guy was experiencing. i like the idea of having an environment variable to opt out of it. would be nice to know why this.setHeader() is not working tho.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
conico974
force-pushed
the
fix/500-cached
branch
from
It seems that sometimes the 500 error page gets cached for some reason.
We don't want that to happen.
Update:
This can happen for page router if your 500 (or 404) page use
getStaticProps.For every route in page router that throws an exception, the cache-control gets overwritten and set as if it was an SSG route. The problem is that it doesn't remove the previous header that was set (including set-cookie). This could lead to session leakage.
Haven't tested in Next 15, this is probably fixed there since they don't override your cache-control anymore ( It might still happen if you don't set any cache-control for the route )
I added an env variable
OPEN_NEXT_DANGEROUSLY_SET_ERROR_HEADERSthat if set to true will disable the cache-control override that we do